Skip to content

fix(docker): enable docker logs for pd/store/server containers#2980

Open
bitflicker64 wants to merge 3 commits intoapache:masterfrom
bitflicker64:fix-docker-logs
Open

fix(docker): enable docker logs for pd/store/server containers#2980
bitflicker64 wants to merge 3 commits intoapache:masterfrom
bitflicker64:fix-docker-logs

Conversation

@bitflicker64
Copy link
Copy Markdown
Contributor

@bitflicker64 bitflicker64 commented Mar 28, 2026

Purpose of the PR

docker logs shows no application output for any HugeGraph container. All JVM logs are silently redirected to files inside the container, requiring manual exec + tail to debug. This PR fixes that.

related comment - #2952 (comment)

Main Changes

Two root causes fixed:

  1. Startup scripts redirected JVM stdout/stderr to a file via >> ${OUTPUT} 2>&1 before Docker could capture it — removed from all 4 scripts
  2. The console appender was defined in all dist log4j2.xml configs but never wired to the root logger — added <appender-ref ref="console"/> to root in all 3 configs

File appenders are kept so non-Docker deployments are unaffected.

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)

Does this PR potentially affect the following parts?

  • Dependencies
  • Modify configurations
  • The public API
  • Other affects (typed here)
  • Nope

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to make HugeGraph PD/Store/Server JVM logs visible via docker logs by stopping shell-level stdout/stderr redirection and ensuring Log4j2 emits to the console.

Changes:

  • Removed >> ... 2>&1 redirections from startup scripts so Docker can capture stdout/stderr.
  • Added console appender references to the Log4j2 root logger in dist configs.
  • Kept file appenders so non-Docker deployments still write log files.

Reviewed changes

Copilot reviewed 3 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
hugegraph-store/hg-store-dist/src/assembly/static/conf/log4j2.xml Adds console appender to root logger.
hugegraph-store/hg-store-dist/src/assembly/static/bin/start-hugegraph-store.sh Stops redirecting JVM output to a file.
hugegraph-server/hugegraph-dist/src/assembly/static/conf/log4j2.xml Adds console appender to root logger (but see comment re AsyncLogger wiring).
hugegraph-server/hugegraph-dist/src/assembly/static/bin/start-hugegraph.sh Stops redirecting daemon/foreground output to a file (but see comment re foreground PID handling).
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh Stops redirecting JVM output to a file.
hugegraph-pd/hg-pd-dist/src/assembly/static/conf/log4j2.xml Adds console appender to root logger.
hugegraph-pd/hg-pd-dist/src/assembly/static/bin/start-hugegraph-pd.sh Stops redirecting JVM output to a file.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 7 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 118 to 122
<loggers>
<root level="INFO">
<appender-ref ref="console"/>
<appender-ref ref="file"/>
</root>
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the console appender to the root logger won’t make logs from loggers with additivity="false" appear in docker logs. In this config, com.alipay.sofa (and other explicitly configured loggers) only write to raft_file/file, so raft/third‑party logs will still be missing from stdout. Consider also adding a console appender-ref to those additivity=false loggers (or revisiting additivity) to fully achieve the Docker logging goal.

Copilot uses AI. Check for mistakes.
Comment on lines 116 to 120
<loggers>
<root level="INFO">
<appender-ref ref="console" />
<appender-ref ref="file" />
</root>
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with the other dist configs, adding console to <root> won’t surface logs from any additivity="false" loggers that only write to raft_file/file. If the goal is comprehensive docker logs output, consider also wiring console into those additivity=false logger definitions (this matches the wiring used in hg-pd-service/src/main/resources/log4j2.xml).

Copilot uses AI. Check for mistakes.
@bitflicker64
Copy link
Copy Markdown
Contributor Author

@imbajin I've wired the console appender into org.apache.hugegraph across all three components. Should I also add it to the third-party loggers or is just the HugeGraph application logger sufficient for the Docker logging goal?

Copy link
Copy Markdown

@vaijosh vaijosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Mar 28, 2026
<root level="INFO">
<appender-ref ref="console" />
<appender-ref ref="file" />
</root>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

‼️ The same completeness gap exists here too: root now has console, but com.alipay.sofa, io.netty, and org.apache.commons still short-circuit to file only because their logger blocks keep additivity="false". Container logs will still miss transport/startup output unless those explicit loggers also opt into console.


<loggers>
<root level="INFO">
<appender-ref ref="console"/>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

‼️ Same issue in the store config: the root console appender does not cover the additivity="false" logger blocks below, so io.netty / org.apache.commons / com.alipay.sofa output is still invisible in docker logs. If the goal is a complete container stream, those explicit loggers need console as well.

<root level="INFO">
<appender-ref ref="console"/>
<appender-ref ref="file"/>
</root>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

‼️ Adding console only to root and org.apache.hugegraph is not enough here: the additivity="false" logger blocks below still bind only file, so startup/runtime messages from org.apache.cassandra, io.netty, com.datastax.driver, com.alipay.sofa, etc. will still be absent from docker logs. Please attach console to the explicit logger blocks that should be visible in containers.

@@ -184,5 +184,4 @@ fi

# Turn on security check
exec ${JAVA} -Dname="HugeGraphServer" ${JVM_OPTIONS} ${JAVA_OPTIONS} -cp ${CLASSPATH}: \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

‼️ Removing the launcher redirection here changes the default binary-package flow too. In a plain install, JVM bootstrap stdout/stderr will no longer land in the logs/*.log file, so the docs that tell users to inspect that file on failure become incomplete. Please gate this on the Docker entrypoint path or preserve the file sink outside containers.

@imbajin
Copy link
Copy Markdown
Member

imbajin commented Apr 1, 2026

@imbajin I've wired the console appender into org.apache.hugegraph across all three components. Should I also add it to the third-party loggers or is just the HugeGraph application logger sufficient for the Docker logging goal?

Seems not necessary for now


# TODO: show the output message in hugegraph-server.sh when start the server
if [[ $DAEMON == "true" ]]; then
echo "Starting HugeGraphServer in daemon mode..."
Copy link
Copy Markdown
Member

@imbajin imbajin Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The safer, lower-risk approach is to add an explicit switch for Docker/stdout mode instead of removing >> ${OUTPUT} 2>&1 from the default startup path.

That keeps the binary-package flow unchanged and still lets containers stream clean logs via docker logs when the switch is enabled.

Default package: start-*.sh -> exec java -> >> logs/*.log 2>&1

Docker package:   entrypoint -> set STDOUT_MODE=true -> exec java -> stdout + console appender

I would keep the file-based behavior by default, then enable stdout only in the Docker entrypoint and wire the console appender in the matching log config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(docker): enable docker logs for pd/store/server containers

4 participants